Skip to content

Feature/comment logic#16

Merged
eeebbaandersson merged 7 commits into
mainfrom
feature/comment-logic
Apr 9, 2026
Merged

Feature/comment logic#16
eeebbaandersson merged 7 commits into
mainfrom
feature/comment-logic

Conversation

@eeebbaandersson

@eeebbaandersson eeebbaandersson commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Add comment support for visa cases: create comments and fetch comments for a specific visa (newest-first).
  • Enhancements
    • Improved error handling with clearer 404 responses for missing resources and explicit 400 responses for invalid requests.
  • Validation
    • Comment creation now rejects empty or null comment text.

- Added CommentRepository and CommentService with create/find comments.
- Added ResourceNotFoundException.
- Updated GlobalExceptionHandler to handle 404 and 400 errors.
# Conflicts:
#	.github/workflows/build-and-test.yml
- Added CommentController with Rest endpoints for creating and fetching comments
@coderabbitai

coderabbitai Bot commented Apr 9, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@eeebbaandersson has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 59 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 59 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 48af4d14-a10a-4202-8eef-a33a933e0ab9

📥 Commits

Reviewing files that changed from the base of the PR and between 490a190 and 936442a.

📒 Files selected for processing (1)
  • src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java
📝 Walkthrough

Walkthrough

Adds a Comment feature: new REST controller endpoints for creating and listing comments, a service that validates and persists comments, a JPA repository with a visa-scoped query, a custom ResourceNotFoundException, and expanded global exception handlers for not-found and bad-request cases.

Changes

Cohort / File(s) Summary
Controller
src/main/java/org/example/visacasemanagementsystem/comment/controller/CommentController.java
New Spring @RestController with POST /api/comments (creates comment) and GET /api/comments/visa/{visaId} (list comments by visaId); constructor-injected CommentService.
Service
src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java
New @Service with createComment(CreateCommentDTO) (validates text, resolves author and visa, maps and saves) and getCommentsByVisaId(Long) (queries repo, checks visa existence, maps to DTOs).
Repository
src/main/java/org/example/visacasemanagementsystem/comment/repository/CommentRepository.java
New JpaRepository<Comment, Long> adding derived query findByVisaIdOrderByCreatedAtDesc(Long visaId).
DTO
src/main/java/org/example/visacasemanagementsystem/comment/dto/CreateCommentDTO.java
Added inline // Todo comment about removing authorId in favor of Spring Security; @NotNull validation for authorId unchanged.
Exceptions & Handlers
src/main/java/org/example/visacasemanagementsystem/exception/ResourceNotFoundException.java, src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java
Adds ResourceNotFoundException; expands global not-found handler to include ResourceNotFoundException and accept RuntimeException; adds IllegalArgumentException handler returning BAD_REQUEST.

Sequence Diagram

sequenceDiagram
    participant Client
    participant CommentController
    participant CommentService
    participant UserRepository
    participant VisaRepository
    participant CommentRepository
    participant Database

    Client->>CommentController: POST /api/comments (CreateCommentDTO)
    CommentController->>CommentService: createComment(dto)
    CommentService->>CommentService: validate dto.text()
    CommentService->>UserRepository: findById(authorId)
    UserRepository->>Database: SELECT user WHERE id=?
    Database-->>UserRepository: user row
    UserRepository-->>CommentService: User entity
    CommentService->>VisaRepository: findById(visaId)
    VisaRepository->>Database: SELECT visa WHERE id=?
    Database-->>VisaRepository: visa row
    VisaRepository-->>CommentService: Visa entity
    CommentService->>CommentRepository: save(comment)
    CommentRepository->>Database: INSERT comment
    Database-->>CommentRepository: saved row
    CommentRepository-->>CommentService: Comment entity
    CommentService-->>CommentController: CommentDTO
    CommentController-->>Client: 201 CREATED (CommentDTO)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through code with tiny paws tonight,

New comments sprouted, tidy and polite.
Controller leads, Service tends the plot,
Repo buries data in a cozy spot,
Exceptions bookmarked — carrots wrapped just right.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Feature/comment logic' is vague and generic, using non-descriptive terms that don't convey meaningful information about what was actually implemented. Use a more specific title that describes the actual functionality added, such as 'Add comment API endpoints and service layer' or 'Implement comment creation and retrieval endpoints'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/comment-logic

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java (1)

61-62: Use readOnly = true for this read-only transaction.

The method only performs queries and DTO mapping with no state mutations. Using @Transactional(readOnly = true) documents intent and enables Hibernate to skip dirty checking, improving performance.

Suggested fix
-    `@Transactional`
+    `@Transactional`(readOnly = true)
     public List<CommentDTO> getCommentsByVisaId(Long visaId) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`
around lines 61 - 62, Change the `@Transactional` annotation on
CommentService.getCommentsByVisaId to a read-only transaction by adding readOnly
= true; locate the method getCommentsByVisaId in CommentService and replace or
update the existing `@Transactional` to `@Transactional`(readOnly = true) so
Hibernate can skip dirty checking and the intent is documented for this
query-only method.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/main/java/org/example/visacasemanagementsystem/comment/controller/CommentController.java`:
- Line 24: The createComment method should enforce Bean Validation on the
request DTO: add the `@Valid` annotation to the CreateCommentDTO parameter in the
createComment(`@RequestBody` CreateCommentDTO dto) signature so Spring triggers
validation of `@NotNull/`@NotBlank on CreateCommentDTO; also import
javax.validation.Valid (or jakarta.validation.Valid depending on project) to
resolve the annotation.

In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`:
- Around line 42-43: The code currently uses dto.authorId() to set comment
ownership (see User author assignment in CommentService), which allows clients
to impersonate others; instead resolve the author from the authenticated context
and ignore the client-supplied id: obtain the current user's id via the app's
auth utility (e.g., SecurityContextHolder/your AuthService method) inside the
CommentService create method, call
userRepository.findById(currentUserId()).orElseThrow(...) to load the User, and
remove/ignore dto.authorId() so ownership is always set from the authenticated
principal rather than client input.

In
`@src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java`:
- Around line 21-24: The method handleNotFoundException in
GlobalExceptionHandler is annotated to handle both EntityNotFoundException and
ResourceNotFoundException but its parameter is typed as EntityNotFoundException;
change the parameter type to a common supertype (e.g., RuntimeException or a
shared base like ApplicationException) so Spring can resolve either exception;
keep the `@ExceptionHandler`({EntityNotFoundException.class,
ResourceNotFoundException.class}) annotation and update the method signature and
any uses of the parameter inside handleNotFoundException to operate on the
chosen supertype.

---

Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`:
- Around line 61-62: Change the `@Transactional` annotation on
CommentService.getCommentsByVisaId to a read-only transaction by adding readOnly
= true; locate the method getCommentsByVisaId in CommentService and replace or
update the existing `@Transactional` to `@Transactional`(readOnly = true) so
Hibernate can skip dirty checking and the intent is documented for this
query-only method.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ac31718c-6c43-48b5-9d9e-b3d5caa56e8e

📥 Commits

Reviewing files that changed from the base of the PR and between 6104784 and 76af3a7.

📒 Files selected for processing (5)
  • src/main/java/org/example/visacasemanagementsystem/comment/controller/CommentController.java
  • src/main/java/org/example/visacasemanagementsystem/comment/repository/CommentRepository.java
  • src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java
  • src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java
  • src/main/java/org/example/visacasemanagementsystem/exception/ResourceNotFoundException.java

Comment on lines +42 to +43
User author = userRepository.findById(dto.authorId())
.orElseThrow(() -> new ResourceNotFoundException("User not found with id: " + dto.authorId()));

@coderabbitai coderabbitai Bot Apr 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Do not trust client-supplied authorId for comment ownership.

Using dto.authorId() directly lets a caller create comments as another user. Resolve author from authenticated context and ignore client-provided identity.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`
around lines 42 - 43, The code currently uses dto.authorId() to set comment
ownership (see User author assignment in CommentService), which allows clients
to impersonate others; instead resolve the author from the authenticated context
and ignore the client-supplied id: obtain the current user's id via the app's
auth utility (e.g., SecurityContextHolder/your AuthService method) inside the
CommentService create method, call
userRepository.findById(currentUserId()).orElseThrow(...) to load the User, and
remove/ignore dto.authorId() so ownership is always set from the authenticated
principal rather than client input.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid point. This issue will be resolved later on when Spring Security is integrated in the project.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eeebbaandersson, understood! That makes sense — resolving the author from the authenticated principal is the right approach once Spring Security is in place (e.g., via SecurityContextHolder.getContext().getAuthentication() or a custom AuthService). At that point, dto.authorId() can be removed from CreateCommentDTO entirely, or kept only for admin-level use cases with proper authorization checks.

I'll note this for future reviews.


✏️ Learnings added
Learnt from: eeebbaandersson
Repo: ithsjava25/visa-case-management-system PR: 16
File: src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java:42-43
Timestamp: 2026-04-09T11:12:57.537Z
Learning: In the `visa-case-management-system` repository, the `authorId` field in `CreateCommentDTO` is currently trusted directly in `CommentService.createComment()` (file: `src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`, around line 42). This is a known security gap intentionally deferred until Spring Security is integrated. Once Spring Security is added, the author should be resolved from the authenticated principal (e.g., via `SecurityContextHolder`) rather than from `dto.authorId()`, and the `authorId` field should be removed from `CreateCommentDTO` or restricted to admin use only.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: Martin-E-Karlsson
Repo: ithsjava25/visa-case-management-system PR: 14
File: src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java:53-53
Timestamp: 2026-04-08T23:24:51.563Z
Learning: In the `visa-case-management-system` repository, password encryption in `UserService.createUser()` (file: `src/main/java/org/example/visacasemanagementsystem/user/service/UserService.java`, line ~53) is intentionally deferred until Spring Security is integrated. The current `user.setPassword(dto.password())` is a temporary plaintext placeholder. When Spring Security is added, a `BCryptPasswordEncoder` bean should be configured and injected into `UserService`, replacing the direct assignment with `passwordEncoder.encode(dto.password())`.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java (2)

65-72: Avoid unconditional pre-check query before fetching comments.

Current flow always does two DB calls (existsById + findByVisaId...). You can fetch comments first and only check visa existence when the result is empty.

💡 Suggested diff
     `@Transactional`(readOnly = true)
     public List<CommentDTO> getCommentsByVisaId(Long visaId) {
-        if (!visaRepository.existsById(visaId)) {
-            throw new ResourceNotFoundException("Visa case not found with id: " + visaId);
-        }
-
-        return commentRepository.findByVisaIdOrderByCreatedAtDesc(visaId)
-                .stream()
-                .map(commentMapper::toDTO)
-                .toList();
+        List<Comment> comments = commentRepository.findByVisaIdOrderByCreatedAtDesc(visaId);
+        if (comments.isEmpty() && !visaRepository.existsById(visaId)) {
+            throw new ResourceNotFoundException("Visa case not found with id: " + visaId);
+        }
+
+        return comments.stream()
+                .map(commentMapper::toDTO)
+                .toList();
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`
around lines 65 - 72, The current implementation in CommentService does an
unconditional existsById check (visaRepository.existsById) before fetching
comments, causing two DB calls; instead call
commentRepository.findByVisaIdOrderByCreatedAtDesc(visaId) first, map the result
to DTOs, and if the returned list is empty then call
visaRepository.existsById(visaId) to decide whether to throw
ResourceNotFoundException("Visa case not found with id: " + visaId) or return an
empty list; update the method that currently calls
commentRepository.findByVisaIdOrderByCreatedAtDesc and visaRepository.existsById
accordingly.

63-64: Use a read-only transaction for the fetch path.

getCommentsByVisaId only reads; marking the transaction read-only better communicates intent and avoids unnecessary persistence-context overhead.

💡 Suggested diff
-    `@Transactional`
+    `@Transactional`(readOnly = true)
     public List<CommentDTO> getCommentsByVisaId(Long visaId) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`
around lines 63 - 64, The getCommentsByVisaId method in CommentService is a
read-only fetch and should be annotated to reflect that; change the
`@Transactional` on getCommentsByVisaId to `@Transactional`(readOnly = true) so the
persistence context is optimized for reads and intent is clear (update the
annotation on the method named getCommentsByVisaId in class CommentService).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`:
- Around line 65-72: The current implementation in CommentService does an
unconditional existsById check (visaRepository.existsById) before fetching
comments, causing two DB calls; instead call
commentRepository.findByVisaIdOrderByCreatedAtDesc(visaId) first, map the result
to DTOs, and if the returned list is empty then call
visaRepository.existsById(visaId) to decide whether to throw
ResourceNotFoundException("Visa case not found with id: " + visaId) or return an
empty list; update the method that currently calls
commentRepository.findByVisaIdOrderByCreatedAtDesc and visaRepository.existsById
accordingly.
- Around line 63-64: The getCommentsByVisaId method in CommentService is a
read-only fetch and should be annotated to reflect that; change the
`@Transactional` on getCommentsByVisaId to `@Transactional`(readOnly = true) so the
persistence context is optimized for reads and intent is clear (update the
annotation on the method named getCommentsByVisaId in class CommentService).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c243695f-a529-4cbb-ae02-f2fa426aa19f

📥 Commits

Reviewing files that changed from the base of the PR and between 76af3a7 and e23b27d.

📒 Files selected for processing (4)
  • src/main/java/org/example/visacasemanagementsystem/comment/controller/CommentController.java
  • src/main/java/org/example/visacasemanagementsystem/comment/dto/CreateCommentDTO.java
  • src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java
  • src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java
✅ Files skipped from review due to trivial changes (1)
  • src/main/java/org/example/visacasemanagementsystem/comment/dto/CreateCommentDTO.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/java/org/example/visacasemanagementsystem/comment/controller/CommentController.java
  • src/main/java/org/example/visacasemanagementsystem/exception/GlobalExceptionHandler.java

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java`:
- Around line 38-40: The createComment method currently dereferences dto without
null-checking, causing an NPE for null payloads; update the
createComment(CreateCommentDTO dto) implementation to first validate dto is not
null (and return/throw a controlled Bad Request/IllegalArgumentException) before
accessing dto.text(), and keep the existing empty-text check; reference the
createComment method and CreateCommentDTO/dto variables when adding this null
guard so the method consistently produces a controlled error for null or blank
inputs.
- Around line 64-67: The method getCommentsByVisaId currently accepts any Long
visaId and can treat zero/negative inputs as valid, causing misleading
repository errors; add input validation at the start of getCommentsByVisaId to
reject null or non-positive visaId (<= 0) and throw a clear
IllegalArgumentException or BadRequest-style exception before calling
commentRepository.findByVisaIdOrderByCreatedAtDesc or visaRepository.existsById;
update any method signature or Javadoc as needed so callers know invalid ids are
rejected and include the symbol names (getCommentsByVisaId, visaId,
commentRepository.findByVisaIdOrderByCreatedAtDesc, visaRepository.existsById,
ResourceNotFoundException) when locating the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a534ef09-fb19-47e0-a189-be818ac6db45

📥 Commits

Reviewing files that changed from the base of the PR and between e23b27d and 490a190.

📒 Files selected for processing (1)
  • src/main/java/org/example/visacasemanagementsystem/comment/service/CommentService.java

- Add input validation to reject non-positive visaID values
- Add null guard for CreateCommentDTO
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extend Global Exception handler with more exceptions Implement missing comment repository, service & controller

1 participant